bt: services: gap_default: Extract GAP service from gatt implementation #94235

rkel · 2025-08-07T15:58:36Z

Remove Generic Access Service implementation from gatt files and move it into separate file in services directory.
The proposed name is just gap_default as it is default implementation that may be switched off by the user in purpose of providing the GAP implementation directly in the application.

In the end it serves the same purpose that #93946 but gives the user total freedom of implementation when needed in the same time being 100% compatible with the current solution.

I was testing it manually and now I can enable and disable default GAP implementation.

The main reason of this PR is to discuss if we wish to go into this solution instead of #93946.

To be done:
[v] - GAP service validation during bt_enable.
[v] - review all configurations that need to be moved to Kconfig.gap_default
[v] - test of the functionality (BabbleSim)
[v] - sample with GAP reimplementation on App side
[ ] - update documentation

alwa-nordic · 2025-08-08T09:50:10Z

subsys/bluetooth/services/Kconfig.gap_default

+# Copyright (c) 2025 Koppel Electronic
+# SPDX-License-Identifier: Apache-2.0
+
+config BT_GAP_DEFAULT


This config name is too generic. The GAP Service is only one component of GAP. I suggest something like BT_GAP_SVC_DEFAULT_IMPL.

alwa-nordic · 2025-08-08T09:54:08Z

subsys/bluetooth/services/Kconfig.gap_default

+	help
+	  Enable the default GATT Generic Access Service.
+	  Enabled by default provides basic GAP functionality.
+	  If disabled user can provide its own implementation in the code.


I think we should say "must provide" and maybe "otherwise the system is not Bluetooth qualification compliant" or similar.

I started it from "must" but then I thought that not everybody cares about being qualification compliant. But the wording like:

If disabled, it is the application responsibility to provide its own implementation of GAP service, otherwise the system is not Bluetooth qualification compliant.

Probably explains everything and not "forces" anybody to do anything - freedom :)

alwa-nordic · 2025-08-08T09:55:43Z

subsys/bluetooth/services/gap_svc_default.c

+				 strlen(name));
+}
+
+#if defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE)


The configs for the GAP Service should be moved Kconfig.gap_default.

Yes. I am just wondering if it Kconfig.gap_default is good place for it. Maybe we should have Kconfig.gap - with all its configurations and then Kconfig.gap_default.

alwa-nordic · 2025-08-08T10:04:45Z

subsys/bluetooth/services/gap_default.c

+/* This checks if the range entered is valid */
+BUILD_ASSERT(!(CONFIG_BT_PERIPHERAL_PREF_MIN_INT > 3200 &&
+	     CONFIG_BT_PERIPHERAL_PREF_MIN_INT < 0xffff));
+BUILD_ASSERT(!(CONFIG_BT_PERIPHERAL_PREF_MAX_INT > 3200 &&
+	     CONFIG_BT_PERIPHERAL_PREF_MAX_INT < 0xffff));
+BUILD_ASSERT(!(CONFIG_BT_PERIPHERAL_PREF_TIMEOUT > 3200 &&
+	     CONFIG_BT_PERIPHERAL_PREF_TIMEOUT < 0xffff));
+BUILD_ASSERT((CONFIG_BT_PERIPHERAL_PREF_MIN_INT == 0xffff) ||
+	     (CONFIG_BT_PERIPHERAL_PREF_MIN_INT <=
+	     CONFIG_BT_PERIPHERAL_PREF_MAX_INT));
+BUILD_ASSERT((CONFIG_BT_PERIPHERAL_PREF_TIMEOUT * 4U) >
+	     ((1U + CONFIG_BT_PERIPHERAL_PREF_LATENCY) *
+	      CONFIG_BT_PERIPHERAL_PREF_MAX_INT));


I'm not sure whether this is not GAP Service specific or even GATT specific. They are used in conn.c. But on the other hand they are defined under "ATT and GATT options". There seems to be a mistake here one way or the other. @jhedberg, should we move these configs out of "ATT and GATT" into a generic GAP space?

It's clearly not right to have these assertions here while the configs are used in conn.c.

Yes, seems like they should move to some GAP-specific place. Btw, some of those build asserts should probably be converted into range directives in Kconfig.

@jhedberg @alwa-nordic - should I just leave this assertions in the gatt.c for the purpose of this PR?

I will do it right now - it looks little odd as now we have only the parameter checking in the gatt.c. But I get it that it is better not to move it to the module that may be disabled.

Copilot

Pull Request Overview

This PR extracts the Generic Access Service (GAP) implementation from the GATT core files into a separate, configurable service module. This allows users to disable the default GAP implementation and provide their own custom implementation when needed.

Key changes:

Creates a new configurable GAP service module in the services directory
Removes GAP implementation from the core GATT files
Maintains 100% compatibility with existing functionality through default enablement

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
subsys/bluetooth/services/gap_default.c	New file containing the extracted GAP service implementation
subsys/bluetooth/services/Kconfig.gap_default	Configuration file for the default GAP service
subsys/bluetooth/services/Kconfig	Includes the new GAP configuration
subsys/bluetooth/services/CMakeLists.txt	Builds the GAP service conditionally
subsys/bluetooth/host/gatt.c	Removes the GAP service implementation
include/zephyr/bluetooth/gap.h	Adds service name definition for consistency

Comments suppressed due to low confidence (1)

Copilot · 2025-08-08T10:06:02Z

subsys/bluetooth/services/gap_svc_default.c

+
+#if defined(CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE)
+	#define GAP_APPEARANCE_PROPS (BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE)
+#if defined(CONFIG_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)


Configuration macro name is inconsistent with the pattern used elsewhere. Should be CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN to match the naming convention.

Suggested change

#if defined(CONFIG_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)

#if defined(CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)

Copilot · 2025-08-08T10:06:03Z

subsys/bluetooth/services/gap_svc_default.c

+	BT_GATT_CHARACTERISTIC(BT_UUID_GAP_DEVICE_NAME,
+			       BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE,
+			       BT_GATT_PERM_READ |
+#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)


Configuration macro name is inconsistent with the pattern used elsewhere. Should be CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_AUTHEN to match the naming convention.

Suggested change

#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)

#if defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_AUTHEN)

Copilot · 2025-08-08T10:06:03Z

subsys/bluetooth/services/gap_svc_default.c

+			       BT_GATT_PERM_READ |
+#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)
+			       BT_GATT_PERM_WRITE_AUTHEN,
+#elif defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)


Configuration macro name is inconsistent with the pattern used elsewhere. Should be CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_ENCRYPT to match the naming convention.

Suggested change

#elif defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)

#elif defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)

rkel · 2025-08-08T11:00:09Z

Can we please discuss now mainly if this is the way we wish to go? If yes - I will work on it little more before changing it into valid PR. Now it is rather proof of concept to discuss this vs #93946. I know we can implement both - but to be honest, any of this two just solves my issues. This one I think gives more freedom to the user, but requires (a little) more work from the application perspective if the user wants to overwrite default functionality.
I am saying "a little more" as you can always just copy default implementation, add any checking and application notification and call it a day.

jhedberg · 2025-08-08T18:36:42Z

Can we please discuss now mainly if this is the way we wish to go?

To me it sounds like a reasonable approach at least, i.e. have a default implementation which can be completely overridden by the app if it wants to.

sonarqubecloud · 2025-08-09T21:37:01Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Thalley · 2025-08-11T11:45:33Z

I generally like and approve of the idea that Zephyr provides a GAP service that works out of the box, similar to DIS, BAS, etc., and where users can choose to define and implement their own (similar to the other services).

2 things we need to consider for this approach is

Unlike (most) other services, GAP service shall always be there on a Bluetooth devices (the only mandatory service IIRC).
Unlike (most) other services, there shall only ever be a single GAP service on a device.

Both of the above can be verified at runtime, where we in bt_enable can verify that the service has been registered, as well as verifying in bt_gatt_service_register that no other service with BT_UUID_GAP has been registered.

jhedberg · 2025-08-14T10:39:29Z

Discussed in the Bluetooth WG meeting. Consensus was to move the GAP service to subsys/bluetooth/services

rkel · 2025-10-01T10:56:01Z

Just a rebase before any changes.

jhedberg · 2025-10-02T09:21:42Z

@rkel please be consistent with the commit subject prefix. Take a look at e.g. git log subsys/bluetooth and just use the same style as the majority of existing commits.

rkel · 2025-10-02T13:10:32Z

@Thalley I had added some verification if there is exactly one GAP service in GATT database.

rkel · 2025-12-01T23:44:41Z

Please note that if #100235 would change before merging - the changes have to migrated to gap_default implementation. But it would not change the idea itself.

Copilot

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

tests/bsim/bluetooth/host/gatt/gap_svc/prj.conf

samples/bluetooth/peripheral_gap_svc/README.rst

subsys/bluetooth/services/Kconfig.gap_svc

tests/bsim/bluetooth/host/gatt/gap_svc/src/central.c

subsys/bluetooth/services/Kconfig.gap_svc

Remove Generic Access Service implementation from gatt files and move it into separate file in services directory. The proposed name is just gap_default as it is default implementation that may be switched off by the user in purpose of providing the GAP implementation directly in the application. Signed-off-by: Radosław Koppel <[email protected]>

…NULL Set the unused argument of the name characteristic to NULL. This removes the reference from GAP service to the hci_core. Signed-off-by: Radosław Koppel <[email protected]>

Implement the validation if there is exactly one GAP service in GATT database. The validation is performed as soon as the GATT database is available and forbids to enable the Bluetooth if the configuration is not compatible with the Bluetooth Specification. The verification may be disabled by the user for better performance or deliberate implementation that is not compliant with the specification. Signed-off-by: Radosław Koppel <[email protected]>

Disable the GATT GAP service validation for the test. Signed-off-by: Radosław Koppel <[email protected]>

The goal of this test is to ensure the default GAP Service can be disabled and the application level GAP service implementation can be provided. Signed-off-by: Radosław Koppel <[email protected]>

Adding a sample that presents how to implement non default GAP service inside the application. Signed-off-by: Radosław Koppel <[email protected]>

rkel · 2025-12-04T08:57:38Z

There are some kconfig options names that copilot yields about and it might have right here (see subsys/bluetooth/services/Kconfig.gap_svc). I marked them as resolved as this is direct copy of old settings and if we decide we should do anything about it I believe it should go to other PR.

sonarqubecloud · 2025-12-04T09:13:20Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
8.7% Duplication on New Code

See analysis details on SonarQube Cloud

rkel mentioned this pull request Aug 7, 2025

bt: host: gatt: Implement GATT GAP callbacks #93946

Closed

rkel requested review from Thalley, alwa-nordic and cvinayak August 7, 2025 16:03

alwa-nordic reviewed Aug 8, 2025

View reviewed changes

alwa-nordic requested a review from Copilot August 8, 2025 10:05

Copilot AI reviewed Aug 8, 2025

View reviewed changes

rkel force-pushed the bt_gap_default_service branch 2 times, most recently from ace01bc to 38b4d8a Compare August 9, 2025 21:29

alwa-nordic added the Bluetooth Review Discussion in the Bluetooth WG meeting required label Aug 14, 2025

jhedberg added area: Bluetooth and removed Bluetooth Review Discussion in the Bluetooth WG meeting required labels Aug 14, 2025

rkel force-pushed the bt_gap_default_service branch from 38b4d8a to 3214e90 Compare October 1, 2025 10:55

rkel force-pushed the bt_gap_default_service branch 3 times, most recently from 9689adc to 5c141cb Compare October 2, 2025 09:14

rkel force-pushed the bt_gap_default_service branch 2 times, most recently from 47420b3 to b34b4eb Compare October 2, 2025 11:05

rkel force-pushed the bt_gap_default_service branch from cb3839c to b511a1e Compare October 3, 2025 05:56

zephyrbot added area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Tests Issues related to a particular existing or missing test area: Samples Samples platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim labels Dec 1, 2025

zephyrbot requested review from HaavardRei, JarmouniA, PavelVPV, aescolar, hermabe, nashif, rugeGerritsen and sjanc December 1, 2025 23:44

zephyrbot assigned jhedberg and alwa-nordic Dec 1, 2025

rkel force-pushed the bt_gap_default_service branch from ba2652a to ebf2cad Compare December 1, 2025 23:45

Copilot finished reviewing on behalf of rkel December 1, 2025 23:46

zephyrbot requested a review from kartben December 1, 2025 23:47

Copilot AI reviewed Dec 1, 2025

View reviewed changes

rkel force-pushed the bt_gap_default_service branch 2 times, most recently from ed5d850 to f9c379e Compare December 3, 2025 23:48

rkel added 6 commits December 4, 2025 09:45

Bluetooth: gap_svc_default: Set user_data for name characteristic to …

a15bda8

…NULL Set the unused argument of the name characteristic to NULL. This removes the reference from GAP service to the hci_core. Signed-off-by: Radosław Koppel <[email protected]>

tests: bluetooth: host: conn: Disable GATT GAP svc validation

8838ee1

Disable the GATT GAP service validation for the test. Signed-off-by: Radosław Koppel <[email protected]>

Tests: Bluetooth: Add 'gap_svc' test

0f4c8d2

The goal of this test is to ensure the default GAP Service can be disabled and the application level GAP service implementation can be provided. Signed-off-by: Radosław Koppel <[email protected]>

samples: bluetooth: peripheral_gap_svc: Adding sample

c34bb12

Adding a sample that presents how to implement non default GAP service inside the application. Signed-off-by: Radosław Koppel <[email protected]>

rkel force-pushed the bt_gap_default_service branch from f9c379e to c34bb12 Compare December 4, 2025 08:45

	#if defined(CONFIG_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)
	#if defined(CONFIG_BT_DEVICE_APPEARANCE_GATT_WRITABLE_AUTHEN)

	#if defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_AUTHEN)
	#if defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_AUTHEN)

	#elif defined(CONFIG_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)
	#elif defined(CONFIG_BT_DEVICE_NAME_GATT_WRITABLE_ENCRYPT)

bt: services: gap_default: Extract GAP service from gatt implementation #94235

Are you sure you want to change the base?

bt: services: gap_default: Extract GAP service from gatt implementation #94235

Conversation

rkel commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkel Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

rkel commented Aug 8, 2025

Uh oh!

jhedberg commented Aug 8, 2025

Uh oh!

sonarqubecloud bot commented Aug 9, 2025

Quality Gate passed

Uh oh!

Thalley commented Aug 11, 2025

Uh oh!

jhedberg commented Aug 14, 2025

Uh oh!

rkel commented Oct 1, 2025

Uh oh!

jhedberg commented Oct 2, 2025

Uh oh!

rkel commented Oct 2, 2025

Uh oh!

rkel commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rkel commented Dec 4, 2025

Uh oh!

sonarqubecloud bot commented Dec 4, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

rkel commented Aug 7, 2025 •

edited

Loading

rkel Aug 8, 2025 •

edited

Loading